Fix policy synchronization failure with database unique constraints in distributed watcher scenarios by Copilot · Pull Request #1593 · apache/casbin

Copilot · 2025-12-07T14:45:59Z

Summary

Fixed a critical bug where Redis WatcherEX notifications would fail to update policies in instances when the database has unique constraints.

Problem

When multiple instances receive watcher notifications, they all try to add the same policy. With unique constraints, only the first instance succeeds at the database level, but all other instances would fail completely and not update their in-memory model, leading to inconsistent state.

Solution

Modified the policy add functions to continue updating the in-memory model even when the adapter fails, as long as:

The policy is valid and can be added to the model
If it's a grouping policy, role link validation passes

If both adapter and model/validation fail, the policy is removed from the model and the adapter error is returned.

Latest Changes (CI Fix)

Refactored nested if blocks to use early returns, reducing code complexity and fixing nestif linter warnings.

Security Summary

CodeQL analysis completed with 0 security alerts. All tests passing. All linter checks passing.

Original prompt

This section details on the original issue you should resolve

<issue_title>[Bug]The Redis Watcherex may fail to update the rules in an instance due to errors caused by database unique constraints when notifying different instances to update rules.</issue_title>
<issue_description>When we update rules with the action of the notification and the watcher Redis WatcherEX,the message will be accepted by all instance,but if we set the unique index in our database,all update action will failed because of the code bellow:
	if e.dispatcher != nil && e.autoNotifyDispatcher {
		return true, e.dispatcher.AddPolicies(sec, ptype, [][]string{rule})
	}

	if e.model.HasPolicy(sec, ptype, rule) {
		return false, nil
	}

	if e.shouldPersist() {
		if err := e.adapter.AddPolicy(sec, ptype, rule); err != nil {
			if err.Error() != notImplemented {
				return false, err
			}
		}
	}

	e.model.AddPolicy(sec, ptype, rule)

	if sec == "g" {
		err := e.BuildIncrementalRoleLinks(model.PolicyAdd, ptype, [][]string{rule})
		if err != nil {
			return true, err
		}
	}

	return true, nil
The function addPolicyWithoutNotify will check if the instance should persist rule to database,and the function will fail because of the unique index,and the function will return,the important action e.model.AddPolicy(sec, ptype, rule) will be skipped , so the rule will not be add to this instance.

I have tried to rewrite the default callback function in Redis WatcherEX,but I can visit the private variable in package casbin.

</issue_description>

Comments on the Issue (you are @copilot in this section)

Fixes [Bug]The Redis Watcherex may fail to update the rules in an instance due to errors caused by database unique constraints when notifying different instances to update rules. #1581

💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

CLAassistant · 2025-12-07T14:46:05Z

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

Co-authored-by: mserico <140243407+mserico@users.noreply.github.com>

When multiple instances receive watcher notifications, all instances try to add the same policy. With unique constraints in the database, the adapter fails on all but the first instance. The fix ensures that policies are still added to the in-memory model even when the adapter fails, allowing instances to stay synchronized. The behavior is: - If adapter fails but policy can be added to model => success (handles duplicates) - If adapter fails and model validation fails => return adapter error - Remove policies don't change (still require to use the current endpoint in the other library without interface changes) Co-authored-by: mserico <140243407+mserico@users.noreply.github.com>

- Fix inefficient string concatenation in toString function - Add missing import for strings package - Fix godot linter issues (missing periods in comments) - Fix ineffassign issues (unused return values) Co-authored-by: mserico <140243407+mserico@users.noreply.github.com>

mserico · 2025-12-07T16:12:33Z

@copilot fix failed CI check:

<html>
<body>
<!--StartFragment--><div class="mt-1 mb-3 px-3" style="box-sizing: border-box; margin-top: 4px !important; margin-bottom: 16px !important; padding-right: 16px !important; padding-left: 16px !important; color: rgb(31, 35, 40); font-family: -apple-system, BlinkMacSystemFont, &quot;Segoe UI&quot;, &quot;Noto Sans&quot;, Helvetica, Arial, sans-serif, &quot;Apple Color Emoji&quot;, &quot;Segoe UI Emoji&quot;; font-size: 14px; font-style: normal; font-variant-ligatures: normal; font-variant-caps: normal; font-weight: 400; letter-spacing: normal; orphans: 2; text-align: start; text-indent: 0px; text-transform: none; widows: 2; word-spacing: 0px; -webkit-text-stroke-width: 0px; white-space: normal; text-decoration-thickness: initial; text-decoration-style: initial; text-decoration-color: initial;"><div class="mx-0 mx-md-1" style="box-sizing: border-box; margin-right: 4px !important; margin-left: 4px !important;"><h2 class="text-bold h4" style="box-sizing: border-box; margin-top: 0px; margin-bottom: 0px; font-size: 16px !important; font-weight: 600 !important;">Annotations</h2><div class="color-fg-muted text-small" style="box-sizing: border-box; color: rgb(89, 99, 110) !important; font-size: 12px !important;">3 errors and 2 warnings</div></div></div>
golangciissues found
--
golangci: internal_api.go#L121`if sec == "g"` has complex nested blocks (complexity: 6) (nestif)
golangci: enforcer_context.go#L496`if sec == "g"` has complex nested blocks (complexity: 6) (nestif)

<!--EndFragment-->
</body>
</html>Annotations
3 errors and 2 warnings
[golangci](https://github.com/casbin/casbin/actions/runs/20006136169/job/57370319926#step:4:28)
issues found
[golangci: internal_api.go#L121](https://github.com/casbin/casbin/pull/1593/files#annotation_42624581123)
`if sec == "g"` has complex nested blocks (complexity: 6) (nestif)
[golangci: enforcer_context.go#L496](https://github.com/casbin/casbin/pull/1593/files#annotation_42624581126)
`if sec == "g"` has complex nested blocks (complexity: 6) (nestif)

Reduced nesting complexity by using early returns instead of nested if blocks in addPoliciesWithoutNotify functions. Changed from nested if-else structure to sequential if checks with early returns, making the code more readable and passing the nestif linter check. Co-authored-by: mserico <140243407+mserico@users.noreply.github.com>

Copilot · 2025-12-07T16:17:44Z

@copilot fix failed CI check:

...

Fixed in commit 3a31759. Refactored the nested if blocks to use early returns instead, which reduces the complexity from 6 to an acceptable level. The nestif linter now passes, and all tests continue to pass.

Initial plan

5e76ea0

Copilot AI assigned Copilot and mserico Dec 7, 2025

Add test to reproduce unique constraint bug with Redis WatcherEX

e930335

Co-authored-by: mserico <140243407+mserico@users.noreply.github.com>

mserico marked this pull request as ready for review December 7, 2025 14:55

Copilot AI and others added 2 commits December 7, 2025 15:05

Copilot AI changed the title ~~[WIP] Fix Redis Watcherex update failure due to unique constraint~~ Fix policy synchronization failure with database unique constraints in distributed watcher scenarios Dec 7, 2025

Copilot AI requested a review from mserico December 7, 2025 15:13

hsluoyz force-pushed the master branch 3 times, most recently from c622095 to b190fce Compare January 6, 2026 07:20

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix policy synchronization failure with database unique constraints in distributed watcher scenarios#1593

Fix policy synchronization failure with database unique constraints in distributed watcher scenarios#1593
Copilot wants to merge 5 commits intomasterfrom
copilot/fix-redis-watcher-update-rules

Copilot AI commented Dec 7, 2025 •

edited

Loading

Uh oh!

CLAassistant commented Dec 7, 2025

Uh oh!

mserico commented Dec 7, 2025

Uh oh!

Copilot AI commented Dec 7, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

Copilot AI commented Dec 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Problem

Solution

Latest Changes (CI Fix)

Security Summary

Comments on the Issue (you are @copilot in this section)

Uh oh!

CLAassistant commented Dec 7, 2025

Uh oh!

mserico commented Dec 7, 2025

Uh oh!

Copilot AI commented Dec 7, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Copilot AI commented Dec 7, 2025 •

edited

Loading